Bun.plugin: check for exception when coercing 'target' to a string by robobun · Pull Request #31560 · oven-sh/bun

robobun · 2026-05-29T05:02:49Z

What

Bun.plugin({ target, setup }) coerced the target option to a string with toStringOrNull() but never checked the throw scope afterwards.

toStringOrNull() returns nullptr and leaves a pending exception when the coercion fails — for example when the value's Symbol.toPrimitive returns an object (which throws a TypeError). Because the pending exception was never checked, setupBunPlugin fell through, built the builder object, and called the user's setup() function with an exception already live. That tripped ExceptionScope::assertNoException():

ASSERTION FAILED: Unexpected exception observed on thread ... at:
Error Exception: Symbol.toPrimitive returned an object
!exception()
.../JavaScriptCore/ExceptionScope.h(61) : void JSC::ExceptionScope::assertNoException()

JSString::value() can also throw (OOM while resolving a rope), so that is checked too.

Repro

const v = { setup() {} };
v[Symbol.toPrimitive] = () => ({});
v.target = v;
Bun.plugin(v); // previously crashed the process

After

The exception propagates to JS as expected instead of crashing:

TypeError: Symbol.toPrimitive returned an object

Valid targets ("node"/"bun"/"browser"), an absent target, and the existing invalid-target error path are unaffected.

Test

Added a regression test in test/js/bun/plugin/plugins.test.ts next to the existing invalid-target test.

Found by Fuzzilli.

The target option was coerced with toStringOrNull() without checking the throw scope afterwards. When the value's Symbol.toPrimitive returns an object, the coercion throws a TypeError but leaves the exception pending, and setupBunPlugin continued on to call the setup function, tripping assertNoException. Check the throw scope after toStringOrNull() and value() so the exception propagates to JS instead of crashing.

robobun · 2026-05-29T05:02:59Z

^{Updated 10:09 PM PT - May 28th, 2026}

❌ @robobun, your commit 3c36de6 has 1 failures in Build #58931 (All Failures):

linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 aarch64-android - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-android - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 aarch64-musl - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-baseline - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-asan - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on :freebsd: aarch64 - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-musl - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 aarch64 - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on :freebsd: x64 - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64-musl-baseline - build-rust
linker stderr: clang++: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument] on 🐧 x64 - build-rust

🧪 To try this PR locally:

bunx bun-pr 31560

That installs a local version of the PR into your bun-31560 executable, so you can run:

bun-31560 --bun

github-actions · 2026-05-29T05:05:55Z

No open issues were found that this PR would fix. The bug (unchecked exception after toStringOrNull() in Bun.plugin() target coercion) appears to have been discovered by Fuzzilli and was not previously reported.

🤖 Generated with Claude Code

coderabbitai · 2026-05-29T05:06:42Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 7e2fde4d-c93f-45eb-b925-c08a4fc7d6ae

📥 Commits

Reviewing files that changed from the base of the PR and between 57d8e38 and 3c36de6.

📒 Files selected for processing (2)

src/jsc/bindings/BunPlugin.cpp
test/js/bun/plugin/plugins.test.ts

Walkthrough

The PR improves exception safety when the Bun plugin processes its target option. The C++ binding now explicitly checks for exceptions after converting the target value to a string, before proceeding to validate it. A new test confirms that exceptions raised during the string coercion are properly propagated to the caller.

Changes

Plugin target exception safety

Layer / File(s)	Summary
Target string conversion exception handling `src/jsc/bindings/BunPlugin.cpp`, `test/js/bun/plugin/plugins.test.ts`	`setupBunPlugin` separates target string conversion from validation by storing the conversion result, checking for exceptions, and then proceeding to extract and validate the string. A new test verifies that exceptions during target coercion are propagated correctly.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: adding exception checking when coercing the 'target' option to a string in Bun.plugin.
Description check	✅ Passed	The description fully covers both required sections: it explains what the PR does (the exception handling fix) and how it was verified (with a specific repro and new regression test).
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

github-actions · 2026-05-29T05:07:49Z

This PR may be a duplicate of:

fix(plugin): propagate exception from Bun.plugin target string coercion #31286 - Both fix the same crash in setupBunPlugin() when coercing the target option to a string throws an exception, by adding RETURN_IF_EXCEPTION checks in BunPlugin.cpp

🤖 Generated with Claude Code

robobun · 2026-05-29T05:08:28Z

Closing as a duplicate of #31286, which already fixes this same crash in setupBunPlugin() (unchecked exception after coercing the plugin target option to a string) and has a full green CI run. Deferring to that PR.

claude

LGTM — straightforward addition of RETURN_IF_EXCEPTION checks following the existing pattern, with a regression test.

Extended reasoning...

Overview

This PR adds two RETURN_IF_EXCEPTION(throwScope, {}) checks in setupBunPlugin (src/jsc/bindings/BunPlugin.cpp) — one after targetValue.toStringOrNull(globalObject) and one after targetJSString->value(globalObject). Both calls can leave a pending JSC exception (the first when Symbol.toPrimitive/toString coercion throws, the second on OOM during rope resolution), and the previous code fell through to call setup() with a live exception, tripping ExceptionScope::assertNoException(). A regression test is added in test/js/bun/plugin/plugins.test.ts next to the existing invalid-target test.

Security risks

None. This is defensive exception propagation in option parsing for Bun.plugin(). It turns a debug-assert crash into a normal thrown TypeError. No new attack surface, auth, crypto, or privilege-related code is touched.

Level of scrutiny

Low. The change is 4 added lines in C++ that apply the exact same RETURN_IF_EXCEPTION macro already used dozens of times in this file (including immediately above for getIfPropertyExists). The refactor from if (auto* x = ...) to a separate declaration + check + if (x) is mechanical and preserves behavior for the non-throwing path. There are no design decisions or behavioral changes for valid inputs.

Other factors

Found by Fuzzilli; PR description includes a clear repro and before/after.
Regression test directly exercises the failing path and asserts the propagated error message.
The bug-hunting system found no issues.
No prior reviewer comments to address.

github-actions Bot added the claude label May 29, 2026

robobun closed this May 29, 2026

robobun deleted the farm/c13c1a2e/fix-bun-plugin-target-tostring-exception branch May 29, 2026 05:08

claude Bot reviewed May 29, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bun.plugin: check for exception when coercing 'target' to a string#31560

Bun.plugin: check for exception when coercing 'target' to a string#31560
robobun wants to merge 1 commit into
mainfrom
farm/c13c1a2e/fix-bun-plugin-target-tostring-exception

robobun commented May 29, 2026

Uh oh!

robobun commented May 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

coderabbitai Bot commented May 29, 2026

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

robobun commented May 29, 2026

Uh oh!

claude Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

robobun commented May 29, 2026

What

Repro

After

Test

Uh oh!

robobun commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

coderabbitai Bot commented May 29, 2026

Walkthrough

Changes

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

robobun commented May 29, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Overview

Security risks

Level of scrutiny

Other factors

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

robobun commented May 29, 2026 •

edited

Loading